-
Notifications
You must be signed in to change notification settings - Fork 380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extract loader-v2-interface crate #4487
Extract loader-v2-interface crate #4487
Conversation
70581d4
to
f35686a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just the naming bit
sdk/loader-instruction/Cargo.toml
Outdated
@@ -0,0 +1,31 @@ | |||
[package] | |||
name = "solana-loader-instruction" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can we call this solana-loader-v2-interface
? the v2
bit so that people are less tempted to use it, especially when they see v3 and v4, and interface
for consistency with the other loader crates, even though there's no state.
I could even be convinced to go with v1
since they share an interface.
sdk/loader-instruction/Cargo.toml
Outdated
|
||
[features] | ||
bincode = ["dep:solana-instruction", "serde"] | ||
dev-context-only-utils = ["bincode"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this feature needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
@yihau can you accept ownership of |
5ffebc6
to
c4bd2c2
Compare
c4bd2c2
to
a30aec7
Compare
loader-v2 program management has been deactivated for a while (a year or longer). Why not remove it entirely? |
would be a breaking change in solana-program and solana-sdk |
Yes, but what about the |
Problem
solana_program::loader_instruction
imposes asolana_program
dep onsolana_transaction_status
.Summary of Changes
solana-loader-instruction
instead ofsolana-loader-interface
because the instruction module is the only thing in the crate